Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix massive performance regression for projects using correct module graphs #411

Merged
merged 17 commits into from
Nov 7, 2024
Merged

Fix massive performance regression for projects using correct module graphs #411

merged 17 commits into from
Nov 7, 2024

Conversation

NullVoxPopuli
Copy link
Member

The aim of this PR is to eventually resolve the massive performance problems ELS has, as reported by: #406

I'm starting with adding some logging, as I just need to figure out where time is being lost and why.

Debugger listening on ws://127.0.0.1:6003/c81ef594-cdb9-4d96-9894-0ff910200d8d
For help, see: https://nodejs.org/en/docs/inspector
[Info  - 6:23:19 PM] lazy init enabled, waiting for config from VSCode
"Initializing Ember Language Server at <repo>"
[Info  - 6:23:19 PM] ELS: executeInitializers
[Info  - 6:23:19 PM] ELS: 1 initializers
[Info  - 6:23:20 PM] ELS: Found 102 roots for <repo>
[Info  - 6:23:20 PM] Initializing new project at <repo>/apps/client with 0 ELS addons.
[Info  - 6:25:29 PM] Ember CLI project added at <repo>/apps/client. (took 128694ms)

😬

So far, it's looking like project.initialize is the problem

@NullVoxPopuli
Copy link
Member Author

NullVoxPopuli commented Oct 24, 2024 •
edited
Loading

it looks like trying to find the "roots" takes the longest:

[Info  - 12:39:34 PM] ELS: executeInitializers
[Info  - 12:39:34 PM] ELS: 1 initializers
[Info  - 12:39:35 PM] ELS: Found 102 roots for <repo>
[Info  - 12:39:35 PM] Initializing new project at <repo>/apps/client with 0 ELS addons.
[Info  - 12:39:35 PM] [collectProjectProviders] +0ms :: Starting : <repo>/apps/client
[Info  - 12:41:50 PM] [collectProjectProviders] +135319ms :: found roots : <repo>/apps/client
[Info  - 12:41:50 PM] [collectProjectProviders] +336ms :: found ELS addons : <repo>/apps/client
[Info  - 12:41:50 PM] [collectProjectProviders] +0ms :: finished crawling dagMap : <repo>/apps/client
[Info  - 12:41:50 PM] Ember CLI project added at <repo>/apps/client. (took 135656ms)
[Info  - 12:44:06 PM] <repo>/apps/client: registry initialized in 136155ms
[Info  - 12:44:06 PM] <repo>/apps/client: script registry initialized in 32ms
[Info  - 12:44:07 PM] <app> registry version mismatch [-1, 0], regenerating...
[Info  - 12:44:07 PM] <app> registry generated in 8ms, new version: 0
[Info  - 12:44:07 PM] Initializing new project at <repo>/apps/documentation with 0 ELS addons.
[Info  - 12:44:07 PM] [collectProjectProviders] +0ms :: Starting : <repo>/apps/documentation
[Info  - 12:44:12 PM] [collectProjectProviders] +4718ms :: found roots : <repo>/apps/documentation
[Info  - 12:44:12 PM] [collectProjectProviders] +66ms :: found ELS addons : <repo>/apps/documentation
[Info  - 12:44:12 PM] [collectProjectProviders] +0ms :: finished crawling dagMap : <repo>/apps/documentation

@NullVoxPopuli
Copy link
Member Author

I have a feeling we're not de-duping dependency crawling based on package.json{name,version}

@NullVoxPopuli NullVoxPopuli changed the title Exploring some extra logging Fix massive performance regression for projects using correct module graphs Oct 24, 2024
@NullVoxPopuli NullVoxPopuli marked this pull request as ready for review October 24, 2024 20:01
@NullVoxPopuli NullVoxPopuli force-pushed the do-not-block-execute-initializers branch from 1306b2f to a9570a8 Compare October 24, 2024 22:36
lifeart
lifeart reviewed Oct 25, 2024
View reviewed changes
src/server.ts Outdated Show resolved Hide resolved
@NullVoxPopuli
Copy link
Member Author 


    TypeError: (0 , node_assert_1.default) is not a function

      67 |               `;

what is going on in this project 😅

lifeart
lifeart reviewed Nov 1, 2024
View reviewed changes
src/utils/layout-helpers.ts Outdated Show resolved Hide resolved
@NullVoxPopuli
Copy link
Member Author

locally: green!
image

lifeart
lifeart reviewed Nov 7, 2024
View reviewed changes
src/utils/layout-helpers.ts Outdated Show resolved Hide resolved
lifeart
lifeart reviewed Nov 7, 2024
View reviewed changes
src/utils/layout-helpers.ts Outdated Show resolved Hide resolved
lifeart
lifeart reviewed Nov 7, 2024
View reviewed changes
src/utils/layout-helpers.ts Outdated Show resolved Hide resolved
lifeart
lifeart reviewed Nov 7, 2024
View reviewed changes
src/utils/logger.ts Outdated Show resolved Hide resolved
lifeart
lifeart reviewed Nov 7, 2024
View reviewed changes
Copy link
Contributor

@lifeart lifeart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job! We really close to land it!

@NullVoxPopuli
Copy link
Member Author

oh wat, it's green. 🤔

@lifeart lifeart merged commit 10c6b28 into master Nov 7, 2024
7 checks passed
@lifeart lifeart deleted the do-not-block-execute-initializers branch November 7, 2024 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lifeart lifeart lifeart approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@NullVoxPopuli @lifeart